libdht: give the shared library a soname; rebuild transmission - #30266
Conversation
5c4670b to
b14bb25
Compare
openwrt-ai
left a comment
openwrt-ai
left a comment
There was a problem hiding this comment.
Reviewed 2 new commits. Both commit messages match their diffs, and I found nothing that blocks a merge.
The soname mechanics check out: ABI_VERSION set inside define Package/libdht is the supported spelling, the versioned package name and the automatic libdht → libdht0 dependency rewrite for consumers are handled by the build system, so transmission needs no DEPENDS edit — CI confirms transmission-daemon (4.1.3-r2) installing libdht0 (2023.03.18~0bbb8f4a-r3). Shipping only libdht.so.0 on target while Build/InstallDev keeps the libdht.so symlink in staging is the right split for -DDHT_LIBRARY=$(STAGING_DIR)/usr/lib/libdht.so, and transmission is indeed the only consumer of libdht in the feed. Passing TARGET_LDFLAGS also has the side benefit of letting -lcrypt resolve against libcrypt-compat in staging, which is what the +USE_GLIBC:libcrypt-compat dependency intends.
Three optional inline notes: one nit on the ABI epoch being hardcoded in four places, one on a version discrepancy in the PR description, and one explaining why the red test-build jobs are unrelated to this change.
Generated by Claude Code
| CATEGORY:=Libraries | ||
| TITLE:=Kademlia Distributed Hash Table (DHT) library | ||
| DEPENDS:= +USE_GLIBC:libcrypt-compat | ||
| ABI_VERSION:=0 |
There was a problem hiding this comment.
nit: the ABI epoch is now written out literally in four places that all have to move together — ABI_VERSION:=0 here, -Wl,-soname,libdht.so.0 and the -o .../libdht.so.0 output name in Build/Compile, the ln -sf libdht.so.0 target, and $(CP) $(PKG_BUILD_DIR)/libdht.so.0 in Package/libdht/install. Since the commit message explicitly invites a future bump ("bump it if the exported surface changes"), a single PKG_ABI_VERSION:=0 near the top with ABI_VERSION:=$(PKG_ABI_VERSION) here and $(PKG_ABI_VERSION) in the rules would make that bump a one-line edit and remove the chance of the soname and the package suffix drifting apart.
That is the pattern the closest in-tree analogue already uses — a hand-set soname plus a versioned install: PKG_ABI_VERSION:=4 at libcurl-gnutls/Makefile:24, ABI_VERSION:=$(PKG_ABI_VERSION) at :44, soname and install rules at :100-:112.
Not a blocker — the current form is correct as written.
Generated by Claude Code
| @@ -9,7 +9,7 @@ include $(TOPDIR)/rules.mk | |||
|
|
|||
| PKG_NAME:=transmission | |||
| PKG_VERSION:=4.1.3 | |||
There was a problem hiding this comment.
nit: the PR description's Testing section reports transmission-daemon-4.1.2-r2.apk, but the package in tree is at 4.1.3 (this line), and CI on this head built transmission-daemon (4.1.3-r2). Looks like the local test build predates the 4.1.3 bump on master — worth correcting the description so the recorded evidence matches what actually gets built, or re-running the check against 4.1.3.
(The libdht0-2023.03.18~0bbb8f4a-r3 figure in the same section does match.)
Generated by Claude Code
| PKG_NAME:=transmission | ||
| PKG_VERSION:=4.1.3 | ||
| PKG_RELEASE:=1 | ||
| PKG_RELEASE:=2 |
There was a problem hiding this comment.
FYI on the five red test-build jobs (x86_64, aarch64_generic, i386_pentium-mmx, arm_cortex-a15_neon-vfpv4, mips_24kc) — they do not appear to be caused by anything in this PR. The failing check is on natpmpc, pulled in as a transmission dependency:
natpmpc: [pass] Binary /usr/bin/natpmpc is stripped
natpmpc: [pass] All linked libraries for /usr/bin/natpmpc are present
natpmpc: [skip] Version check override
natpmpc: No executables in the package provided version 20230423
natpmpc: Generic tests failed
natpmpc prints its usage screen without the version string and net/libnatpmp carries no test-version.sh override, so the generic version check fails for it on any PR that causes transmission's dependency set to be rebuilt. All four transmission subpackages and libdht0 itself pass their generic tests in the same runs — including All linked libraries for /usr/bin/transmission-daemon are present with libdht0 installed, which is the part this PR actually changes. Flagging only so the red X is not mistaken for a soname regression; the fix belongs in net/libnatpmp, not here.
Generated by Claude Code
The library installed as a bare libdht.so with no SONAME, so consumers recorded the filename as their DT_NEEDED and there was no way to express an ABI break. Build it as libdht.so.0 with a matching soname and declare ABI_VERSION, so the package is versioned like any other shared library. jech/dht makes no ABI promise of its own, so 0 is a packaged ABI epoch: bump PKG_ABI_VERSION if the exported surface changes. That epoch is now a single variable referenced from ABI_VERSION, the soname, the output filename and the symlink target, rather than repeated literally in all four places, matching the pattern already used by libcurl-gnutls. Also pass TARGET_CFLAGS/TARGET_CPPFLAGS/TARGET_LDFLAGS to the compile and link, which the hand-written rules had skipped, and fix PKG_LICENSE_FILES: jech/dht ships LICENCE, not LICENSE, so the declared file was never found. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
libdht now installs libdht.so.0 with a soname and an ABI_VERSION, so the package it depends on is libdht0 rather than libdht. Bump PKG_RELEASE to force the rebuild that picks that up. No other change is needed: DHT_LIBRARY still points at the libdht.so development symlink, and the resulting binary records the soname, so transmission-daemon links libdht.so.0 and depends on libdht0. Signed-off-by: Daniel Golle <daniel@makrotopia.org>
b14bb25 to
7bd3e9f
Compare
2 participants
Maintainer
Myself (maintainer of both packages).
Description
libdhtinstalled a barelibdht.sowith noSONAME, so consumersrecorded the plain filename in
DT_NEEDEDand there was no way toexpress an ABI break. This builds it as
libdht.so.0with a matchingsoname and declares
ABI_VERSION:=0, so it is versioned like any othershared library in the feed.
jech/dht makes no ABI promise of its own, so 0 is a packaged ABI epoch,
to be bumped if the exported surface changes. That epoch is now a single
PKG_ABI_VERSIONvariable referenced fromABI_VERSION, the soname, theoutput filename and the symlink target, so a future bump is a one-line
edit instead of four matching edits (thanks to the review nit pointing at
libcurl-gnutlsfor the pattern).Two smaller fixes ride along in the same commit:
TARGET_CFLAGS/TARGET_CPPFLAGS/TARGET_LDFLAGSare now passedto the hand-written compile and link rules, which had skipped them.
PKG_LICENSE_FILESsaidLICENSE, but jech/dht shipsLICENCE, sothe declared licence file was never actually found.
transmissionis the only consumer in the feed. It needs no sourcechange --
DHT_LIBRARYstill points at thelibdht.sodevelopmentsymlink -- but its
PKG_RELEASEis bumped so it is rebuilt against theversioned library.
Testing
Built for
aarch64_cortex-a53(mediatek/filogic):libdht0-2023.03.18~0bbb8f4a-r3.apk,SONAME: libdht.so.0CI (x86_64 run on this branch) built and generic-tested the rebuilt
transmission-daemon-4.1.3-r2.apk(master had moved from 4.1.2 to 4.1.3by the time CI ran, correcting the mismatch an earlier revision of this
description had against the actual 4.1.3-r2 build): it depends on
libdht0, "All linked libraries for /usr/lib/libdht.so.0 are present",and "SONAME link for /usr/lib/libdht.so.0 is correct".
https://github.com/openwrt/packages/actions/runs/31609574985/job/94157294230
Not built for other architectures or libcs.